Skip to content

fix(ci): don't fail e2e job when dev server already exited#4054

Open
aterga wants to merge 2 commits into
mainfrom
claude/fix-e2e-dev-server-cleanup
Open

fix(ci): don't fail e2e job when dev server already exited#4054
aterga wants to merge 2 commits into
mainfrom
claude/fix-e2e-dev-server-cleanup

Conversation

@aterga

@aterga aterga commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

The e2e-playwright matrix' "Stop dev server" cleanup step runs:

- name: Stop dev server
  if: ${{ always() }}
  run: kill ${{ steps.dev-server-start.outputs.dev_server_pid }}

under bash -e. The dev server is started as a backgrounded npm run dev | tee … & pipeline, and the captured $! PID is frequently already reaped by the time this cleanup step runs. kill then exits non-zero (kill: (NNNN) - No such process), and because the step runs under bash -e, the whole job fails — after every test has already passed.

This produces spurious red e2e shards (observed repeatedly on desktop, 2_6 and mobile, 2_6) on PRs that don't touch any test or app code, e.g. a docs/scripts-only change. Example log tail:

  31 passed (5.4m)
Run kill 3405
kill: (3405) - No such process
##[error]Process completed with exit code 1.

Fix

Make the cleanup non-fatal:

run: kill ${{ steps.dev-server-start.outputs.dev_server_pid }} 2>/dev/null || true

Kill semantics are unchanged — anything still alive is reaped by the runner's orphan-process cleanup (visible as Terminate orphan process: … at job end). The step simply no longer turns a green run red. This is the only kill-based cleanup step in the workflows (verified), so one edit covers all e2e shards.

Testing

  • yaml.safe_load parses the workflow.
  • The change is confined to the cleanup step; it cannot affect test outcomes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CGvgPmEVcgyUP2mAnwnDcj


Generated by Claude Code

The e2e-playwright matrix' "Stop dev server" cleanup step ran
`kill <dev_server_pid>` under `bash -e`. The dev server is launched as a
backgrounded `npm run dev | tee` pipeline, whose `$!` PID is frequently
already reaped by the time cleanup runs — so `kill` exits non-zero with
"No such process" and fails the whole job even though every test passed.
This produced spurious red shards (e.g. desktop/mobile 2_6) on unrelated PRs.

Make the cleanup non-fatal: `kill ... 2>/dev/null || true`. Anything still
alive is reaped by the runner's orphan-process cleanup, so kill semantics are
unchanged — the step just no longer turns a green run red.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGvgPmEVcgyUP2mAnwnDcj
@aterga aterga marked this pull request as ready for review June 23, 2026 10:34
@aterga aterga requested a review from a team as a code owner June 23, 2026 10:34
Copilot AI review requested due to automatic review settings June 23, 2026 10:34
@zeropath-ai

zeropath-ai Bot commented Jun 23, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to b35ca80.

Security Overview
Detected Code Changes
Change Type Relevant files
Other ► .github/workflows/canister-tests.yml
    Improve robustness of dev server shutdown in CI

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the e2e-playwright GitHub Actions workflow cleanup so that stopping the background dev server can’t fail an otherwise-successful E2E shard, eliminating spurious red CI runs caused by kill returning non-zero when the process has already exited.

Changes:

  • Makes the “Stop dev server” step non-fatal by ignoring kill errors (2>/dev/null || true).
  • Adds inline documentation explaining why this cleanup must not fail under bash -e.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants